Conversation
…ization edit form
…d confirmation email
…uantity in DistributionPdf
…e proper unit display in CSV export
…ions and improve quantity calculations
…ganyni1/human-essentials into 5251-remove-enable-packs
…o quantity handling
|
This is going to take a couple of sessions to do the functional check -- we basically have to check every packs-related thing (and there are a lot of them) |
|
I understand, please let me know if I need to change anything. |
|
Looks good to me -- over to @dorner for technical insight. |
app/models/partners/item_request.rb
Outdated
| else | ||
| item&.name || name | ||
| end | ||
| unit_text = request_unit.present? ? request_unit.pluralize(quantity_override || quantity.to_i) : "items" |
There was a problem hiding this comment.
This changes the behavior in that it adds " - items" where it didn't do that before.
@cielf thoughts?
There was a problem hiding this comment.
I think you're right -- the "items" is unneeded.
| row += Array.new(item_headers.size, 0) | ||
|
|
||
| request.item_requests.each do |item_request| | ||
| item_name = item_request.item.present? ? item_request.name_with_unit(0) : DELETED_ITEMS_COLUMN_HEADER |
There was a problem hiding this comment.
Why did this have to change?
| ["Item 2", 30, 100], | ||
| ["Item 3", 50, ""], | ||
| ["Item 4", 120, ""], | ||
| ["Item 4", "120 packs", ""], |
| end | ||
|
|
||
| after do | ||
| Flipper.disable(:enable_packs) |
There was a problem hiding this comment.
This block can be removed entirely now.
| "2T Diapers -- UPDATED", | ||
| "3T Diapers", | ||
| "4T Diapers", | ||
| "4T Diapers - packs", |
There was a problem hiding this comment.
Why did we have to add this?
| let(:organization) { create(:organization) } | ||
| let(:fake_organization_items) { instance_double('organization.items') } | ||
| let(:fake_organization_item) { instance_double(Item, id: 99_999, save!: -> {}) } | ||
| let(:fake_organization_item) { instance_double(Item, id: 99_999, save!: -> {}, sync_request_units!: true) } |
There was a problem hiding this comment.
Same question - if all we're doing is removing the feature flag, then tests should be identical with the exception of any tests that had the flag off.
|
@yahyaelganyni1 It looks like there are spots @dorner pointed out that need some thought/changes. Are you available to continue with this PR? |
|
Sorry for not responding to @dorner points, and I will start working on them by next week. |
4 participants
Resolves #5251
Description
This change removes the Flipper feature flag for "enable packs" since the feature is now permanently enabled in production. The work involves:
Type of change
How Has This Been Tested?
Screenshots
N/A (backend changes only)